Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow empty public keys #371

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

cmwaters
Copy link

@cmwaters cmwaters commented Jan 17, 2024

Description

Empty public keys would throw an error when calling SetSignatures. Public keys are optional and only needed in the first transaction to set it in state.

This should not break consensus and thus is okay as a minor release


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@cmwaters cmwaters changed the title feat: allow empty public keys fix: allow empty public keys Jan 17, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2024

Codecov Report

Attention: 52 lines in your changes are missing coverage. Please review.

Comparison is base (e04d485) 65.55% compared to head (fdfb76a) 65.54%.
Report is 4 commits behind head on release/v0.46.x-celestia.

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                      @@
##           release/v0.46.x-celestia     #371      +/-   ##
============================================================
- Coverage                     65.55%   65.54%   -0.01%     
============================================================
  Files                           666      666              
  Lines                         71180    71213      +33     
============================================================
+ Hits                          46661    46680      +19     
- Misses                        21936    21945       +9     
- Partials                       2583     2588       +5     
Files Coverage Δ
x/upgrade/keeper/keeper.go 83.72% <100.00%> (ø)
x/auth/tx/builder.go 82.35% <70.00%> (+0.29%) ⬆️
x/params/types/consensus_params.go 0.00% <0.00%> (ø)
x/auth/client/cli/tx_multisign.go 0.00% <0.00%> (ø)
baseapp/abci.go 59.93% <11.11%> (-0.67%) ⬇️
baseapp/options.go 56.66% <0.00%> (-2.99%) ⬇️
baseapp/params.go 80.00% <0.00%> (-20.00%) ⬇️
baseapp/baseapp.go 73.43% <5.55%> (-3.03%) ⬇️

... and 1 file with indirect coverage changes

@cmwaters cmwaters merged commit 0244d0c into release/v0.46.x-celestia Jan 19, 2024
33 checks passed
@cmwaters cmwaters deleted the cal/allow-empty-pubkeys branch January 19, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants